Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refcount redirection close #106

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kvojacheck
Copy link

Proposed reference counting workaround to issue #105
Please kindly review, since I'm quite new to Windows driver developement.

Refactor CUsbDkFilterStrategy::OnClose() in a way that the process id
can be passed from the beginning of the call-chain.
This is just a technical change needed for the next commit.
Add reference counting, since the previously used 'PID-based' method
might not work reliably: for example, if the program doing redirection
is forcefully closed from the task manager, the USB redirection could
remain stuck.
After this commit, we are always stopping redirection if the reference
count drops to zero.
@ybendito
Copy link
Collaborator

@kvojacheck What happens in following flow: app1 redirects the device and uses it, app2 opens the device and closes it?
as pid == 0 means 'match all' will this stop the redirection?

@ybendito
Copy link
Collaborator

Probably the proper thing is to use file context (WDFFILECONTEXT) instead of process identifier

@kvojacheck
Copy link
Author

@kvojacheck What happens in following flow: app1 redirects the device and uses it, app2 opens the device and closes it? as pid == 0 means 'match all' will this stop the redirection?

At the time app2 closes the device, the refcount is still 1 (since app1 is still using the device), therefore we would go to the PID-based identification as before: as the returned PID value by PsGetCurrentProcessId is likely corresponds to app2 (and most likely not app1), the closure of app2 wouldn't cause the removal of the redirection.

@kvojacheck
Copy link
Author

Probably the proper thing is to use file context (WDFFILECONTEXT) instead of process identifier

Thank you for the tip, I will definitely look into this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants